DLL-inteface for operator<< (MSVC compatibility)#114
DLL-inteface for operator<< (MSVC compatibility)#114dvetutnev wants to merge 14 commits intodocopt:masterfrom
Conversation
dvetutnev
changed the title
jaredgrubb
left a comment
jaredgrubb
left a comment
There was a problem hiding this comment.
In general, I prefer functional changes to be split into a separate commit from formatting or style changes.
docopt.cpp
Outdated
| #include <cstddef> | ||
|
|
||
| using namespace docopt; | ||
| namespace docopt { |
There was a problem hiding this comment.
Can you explain why you're doing this? I don't necessarily oppose it but would like to understand the justification. (Especially because you labeled this as something to do with GCC, which shouldn't be an issue).
One reason I like the "docopt::" in the definitions is that it catches bugs (if a function gets defined that didn't have a declaration).
| /// @throws DocoptExitVersion if 'version' is true and the user has passed the '--version' argument | ||
| /// @throws DocoptArgumentError if the user's argv did not match the usage patterns | ||
| std::map<std::string, value> DOCOPT_API docopt_parse(std::string const& doc, | ||
| DOCOPT_API |
There was a problem hiding this comment.
I'm a little nervous about swapping the order of these (the return type and the annotation). Was this intentional?
There was a problem hiding this comment.
__declspec(dllexport)/__declspec(dllimport) should be of left-side */& (MSVC specfic). This change for one-style with operator<<
docopt_api.h
Outdated
| // docopt | ||
| // | ||
| // Created by Dmitriy Vetutnev on 2019-09-05. | ||
| // Copyright (c) 2019 Dmitriy Vetutnev. All rights reserved. |
There was a problem hiding this comment.
Can we remove the attribution and copyright here? I don't think we necessarily need any copyright attribution outside of the top one. (In other words, I'm not asking you to change it match the docopt.h header, just leave only the lines 1-4 and delete 5 and 6).
docopt_value.h
Outdated
|
|
||
| /// Write out the contents to the ostream | ||
| std::ostream& operator<<(std::ostream&, value const&); | ||
| DOCOPT_API std::ostream& operator<<(std::ostream&, const value&); |
There was a problem hiding this comment.
Please change back to value const& (the style for this library is east-const)
|
This looks good. Can you vouch that MSVC is happy with this? I have no Windows machines myself to try on. |
|
Yes. I`am build on Windows 7 MSVC 2017. |
2 participants
No description provided.